Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[xdoctest][task skip_no_reason 8-11] add skip reason for jit sample code #57676

Merged
merged 3 commits into from
Sep 27, 2023
Merged

[xdoctest][task skip_no_reason 8-11] add skip reason for jit sample code #57676

merged 3 commits into from
Sep 27, 2023

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Sep 24, 2023 •
edited
Loading

PR types

Others

PR changes

Others

Description

为之前没有说明 skip reason 的示例代码添加 skip reason，本 PR 为 jit 下所有相关代码添加

@megemini @sunzhongkai588

Related links

PCard-66972

@paddle-bot
Copy link

paddle-bot bot commented Sep 24, 2023

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

SigureMo
SigureMo commented Sep 25, 2023
View reviewed changes
python/paddle/jit/api.py Outdated
@@ -1368,7 +1369,7 @@ def load(path, **configs):
.. code-block:: python
:name: code-example1
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

奇怪，code-example2 一直不过，但本地可以过 @megemini

image

看着像多进程导致的问题？但我用的是最新版 convert-doctest 呀

另外 TracedLayer 三个报错先不用管，TracedLayer 目前已经不用了

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗯 ... ... 之前其实一直担心，使用多进程可能会对 paddle 本身有一定副作用 ～

这里 paddle 内部也用了多进程（应该是加载数据用的），不能嵌套进程，即使按照报错信息不用 daemon，也会因为不能 pickle 而报错 ～

有个办法，加个全局 directive，让这个用例不在子进程运行，看看行不？

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里 paddle 内部也用了多进程（应该是加载数据用的），不能嵌套进程，即使按照报错信息不用 daemon，也会因为不能 pickle 而报错 ～

可是 convert-doctest 为什么没问题呢？这俩按理说是一样的呀？

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我在 aistudio 上也报错 ～

你是在本地 mac 运行的？

不同系统对于 multiprocessing 处理不一样 ... ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paddle 目前强制用的 fork：

Paddle/python/paddle/reader/decorator.py

Lines 29 to 42 in 395ffbf

# On macOS, the 'spawn' start method is now the default in Python3.8 multiprocessing,
# Paddle is currently unable to solve this, so forces the process to start using
# the 'fork' start method.
#
# TODO: This solution is not good, because the fork start method could lead to
# crashes of the subprocess. Figure out how to make 'spawn' work.
#
# For more details, please refer to
# https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods
# https://bugs.python.org/issue33725
if sys.version_info >= (3, 8) and sys.platform == 'darwin':
fork_context = multiprocessing.get_context('fork')
else:
fork_context = multiprocessing

出错的 _DataLoaderIterMultiProcess 也是用的默认的 multiprocessing.Process，linux 下是 fork，mac 下是 spawn ～

fork is fast, unsafe, and maybe bloated，另参考 Fork vs Spawn in Python Multiprocessing, 所以当时我是强制用的 spawn。

我试过用 fork，虽然快，但是问题更多 ... ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你是在本地 mac 运行的？

嗯嗯对

有个办法，加个全局 directive，让这个用例不在子进程运行，看看行不？

那看样子只能这样了～

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR： #57692 🤔

megemini
megemini reviewed Sep 26, 2023
View reviewed changes
Copy link
Contributor

@megemini megemini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

把那个 load 的冲突解决一下应该就没问题了吧 ？～

@SigureMo
Copy link
Member Author

把那个 load 的冲突解决一下应该就没问题了吧 ？～

奇怪，我 rerun CI 了，但好像没有效果，我 merge 一下重新触发试试吧

@luotao1 luotao1 self-assigned this Sep 26, 2023
@luotao1 luotao1 mentioned this pull request Sep 26, 2023
Closed
sunzhongkai588
sunzhongkai588 approved these changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SigureMo SigureMo merged commit 363d8b0 into PaddlePaddle:develop Sep 27, 2023
@SigureMo SigureMo deleted the xdoctest/skip-reason/jit branch September 27, 2023 04:55
@yuchen202 yuchen202 mentioned this pull request Oct 11, 2023
Closed
Frida-a pushed a commit to Frida-a/Paddle that referenced this pull request Oct 14, 2023
@SigureMo @Frida-a
[xdoctest][task skip_no_reason 8-11] add skip reason for jit sample c…
a5b2194 
…ode (PaddlePaddle#57676)
jiahy0825 pushed a commit to jiahy0825/Paddle that referenced this pull request Oct 16, 2023
@SigureMo @jiahy0825
[xdoctest][task skip_no_reason 8-11] add skip reason for jit sample c…
ec29db9 
…ode (PaddlePaddle#57676)
danleifeng pushed a commit to danleifeng/Paddle that referenced this pull request Nov 14, 2023
@SigureMo
[xdoctest][task skip_no_reason 8-11] add skip reason for jit sample c…
0a1334b 
…ode (PaddlePaddle#57676)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@megemini megemini megemini left review comments

@sunzhongkai588 sunzhongkai588 sunzhongkai588 approved these changes

Assignees

@luotao1 luotao1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SigureMo @megemini @sunzhongkai588 @luotao1